feat(cmd): add service mode to db nuke command #5185
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // removeContent removes all content from a directory | ||
| func (s *NukeService) removeContent(path string) error { | ||
| dir, err := os.Open(path) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
To fix this vulnerability, we must validate the DataDir value before using it to construct file system paths. The best approach is to restrict DataDir to a safe, pre-configured base directory (e.g., the application's data directory), and ensure that the resolved path is within this directory. This can be done by resolving the absolute path of the user-supplied DataDir, joining it with the intended subdirectory, and then checking that the resulting path is still within the allowed base directory. If not, the request should be rejected.
The changes should be made in pkg/api/nuke.go, specifically in the StartNuke and/or executeNuke methods, to validate req.DataDir before it is used. We should define a safe base directory (either as a constant or configurable value), and ensure that all operations are restricted to subdirectories of this base. We may also want to reject absolute paths, paths containing "..", or paths with path separators if only a single directory name is expected.
We will need to import "strings" (if not already present) and use filepath.Abs and strings.HasPrefix for validation.
| @@ -12,2 +12,3 @@ | ||
| "path/filepath" | ||
| "strings" | ||
| "sync" | ||
| @@ -54,2 +55,5 @@ | ||
|
|
||
| // safeBaseDir is the directory under which nuking is allowed. | ||
| const safeBaseDir = "/var/lib/bee" // Change this to your actual safe data directory | ||
|
|
||
| // NewNukeService creates a new nuke service | ||
| @@ -83,2 +87,18 @@ | ||
| func (s *NukeService) executeNuke(req NukeRequest) { | ||
| // Validate that DataDir is within safeBaseDir | ||
| absBase, err := filepath.Abs(safeBaseDir) | ||
| if err != nil { | ||
| s.error = fmt.Errorf("failed to resolve safe base dir: %w", err) | ||
| return | ||
| } | ||
| absDataDir, err := filepath.Abs(req.DataDir) | ||
| if err != nil { | ||
| s.error = fmt.Errorf("failed to resolve data dir: %w", err) | ||
| return | ||
| } | ||
| if !strings.HasPrefix(absDataDir, absBase) { | ||
| s.error = fmt.Errorf("invalid data_dir: must be within %s", safeBaseDir) | ||
| return | ||
| } | ||
|
|
||
| defer func() { | ||
| @@ -115,3 +135,4 @@ | ||
| for _, dir := range dirsToNuke { | ||
| err := s.removeContent(filepath.Join(req.DataDir, dir)) | ||
| // Use absDataDir instead of req.DataDir | ||
| err := s.removeContent(filepath.Join(absDataDir, dir)) | ||
| if err != nil { |
| } | ||
|
|
||
| for _, sub := range subpaths { | ||
| err = os.RemoveAll(filepath.Join(path, sub)) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
To fix the problem, we must validate the user-supplied DataDir before using it to construct file paths. The best approach is to restrict DataDir to a safe, expected base directory (e.g., a configured root data directory), and ensure that the resolved path is within this directory. This can be done by resolving the absolute path of the user-supplied directory and checking that it is a subdirectory of the allowed base directory. If not, the request should be rejected.
Steps:
- Define a constant or configuration for the allowed base directory (e.g.,
/var/lib/beeor similar). - In
StartNuke, before starting the operation, resolve the absolute path ofreq.DataDir(joined with the base directory if needed). - Check that the resolved path is within the allowed base directory using
filepath.HasPrefixor similar logic. - If the check fails, return an error and do not proceed.
- Only proceed with the nuke operation if the check passes.
Required changes:
- Add a constant for the allowed base directory in
pkg/api/nuke.go. - Add validation logic in
StartNuketo check thatreq.DataDiris within the allowed base directory. - Optionally, update the error messages to inform the user if the path is invalid.
| @@ -12,2 +12,3 @@ | ||
| "path/filepath" | ||
| "strings" | ||
| "sync" | ||
| @@ -29,2 +30,5 @@ | ||
|
|
||
| // AllowedBaseDir is the only directory under which nuking is allowed. | ||
| const AllowedBaseDir = "/var/lib/bee" // TODO: set this to your actual data root directory | ||
|
|
||
| // NukeStatus represents the current status of the nuke operation | ||
| @@ -70,2 +74,24 @@ | ||
| } | ||
|
|
||
| // Validate that DataDir is within AllowedBaseDir | ||
| absBase, err := filepath.Abs(AllowedBaseDir) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve allowed base dir: %w", err) | ||
| } | ||
| absDataDir, err := filepath.Abs(req.DataDir) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve data dir: %w", err) | ||
| } | ||
| // Ensure trailing separator for prefix check | ||
| baseWithSep := absBase | ||
| if !strings.HasSuffix(baseWithSep, string(os.PathSeparator)) { | ||
| baseWithSep += string(os.PathSeparator) | ||
| } | ||
| dataWithSep := absDataDir | ||
| if !strings.HasSuffix(dataWithSep, string(os.PathSeparator)) { | ||
| dataWithSep += string(os.PathSeparator) | ||
| } | ||
| if absDataDir != absBase && !strings.HasPrefix(dataWithSep, baseWithSep) { | ||
| return fmt.Errorf("data_dir must be within allowed base directory: %s", AllowedBaseDir) | ||
| } | ||
|
|
Checklist
Description
Open API Spec Version Changes (if applicable)
Motivation and Context (Optional)
Related Issue (Optional)
Screenshots (if appropriate):